Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not print in bold in print_with_color + other color tweaks #18480

Closed
wants to merge 2 commits into from
Closed

Do not print in bold in print_with_color + other color tweaks #18480

wants to merge 2 commits into from

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Sep 13, 2016 •
edited
Loading

  • Changes print_with_color and company to not print things in bold by default.
  • As a consequence of above, changes the default color for warnings and error printing to use light_red which is the same color as bold + red previously was on most terminals.
  • with_output_color does no longer reset all the color state (does not print a \e[0m in the end) but instead "pops" off the last one. This means you can nest a bold style and a color style in with_output_color.
  • Makes the "ERROR", "INFO", "WARNING" part bold.

image

image

tkelman
tkelman reviewed Sep 13, 2016
View reviewed changes
doc/stdlib/io-network.rst
@@ -508,7 +508,7 @@ Text I/O

Print strings in a color specified as a symbol.

``color`` may take any of the values ``:normal``\ , ``:bold``\ , ``:black``\ , ``:blue``\ , ``:cyan``\ , ``:green``\ , ``:magenta``\ , ``:red``\ , ``:white``\ , or ``:yellow``\ .
``color`` may take any of the values ``:normal``\ , ``:bold``\ , ``:black``\ , ``:blue``\ , ``:cyan``\ , ``:green``\ , ``:italics``\ , ``:magenta``\ , ``:red``\ , ``:underline``\ , ``:white``\ , or ``:yellow``\ .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't we have this set up to list styles like normal/bold first, then colors alphabetically?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, will fix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated this + other comment

@andreasnoack
Copy link
Member

Great. Maybe also disable bold for the input text.

@KristofferC
Copy link
Member Author

That is a too controversial of a change @andreasnoack to handle in this PR :P

@andreasnoack
Copy link
Member

I think the controversial part was the printing.

@KristofferC
Copy link
Member Author

KristofferC commented Sep 13, 2016 •
edited
Loading

This is only changed for printing info, warning, error messages and explicit calls to print_with_color. showed variables are still bolded

image

@KristofferC
Copy link
Member Author

This has the effect that if you set the color of JULIA_INPUT_COLOR then the input will no longer be bold... And the way things are done now, there is no nice way to specify that you want both bold and a specific color.

tkelman
tkelman reviewed Sep 13, 2016
View reviewed changes
doc/stdlib/io-network.rst
@@ -508,7 +508,7 @@ Text I/O

Print strings in a color specified as a symbol.

``color`` may take any of the values ``:underline``\ , ``:normal``\ , ``:italics``\ , ``:bold``\ , ``:black``\ , ``:blue``\ , ``:cyan``\ , ``:dark_gray``\ , ``:green``\ , ``:light_blue``\ , ``:light_cyan``\ , ``:light_green``\ , ``:light_magenta``\ , ``:light_red``\ , ``:light_yellow``\ , ``:magenta``\ , ``:red``\ , ``:white``\ , or ``:yellow``\ .
``color`` may take any of the values ``:underline``\ , ``:normal``\ , ``:italics``\ , ``:bold``\ , ``:black``\ , ``:blue``\ , ``:cyan``\ , ``:dark_gray``\ , ``:default``\ , ``:green``\ , ``:light_blue``\ , ``:light_cyan``\ , ``:light_green``\ , ``:light_magenta``\ , ``:light_red``\ , ``:light_yellow``\ , ``:magenta``\ , ``:red``\ , ``:white``\ , or ``:yellow``\ .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should default also count as formatting rather than color?

@tkelman
Copy link
Contributor

tkelman commented Sep 13, 2016

are the light_ colors equivalent to bold, or not? kind of unfortunate to lose the ability to ask for a bold configuration if you want it

@StefanKarpinski StefanKarpinski added the REPL Julia's REPL (Read Eval Print Loop) label Sep 13, 2016
@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Sep 13, 2016
@KristofferC
Copy link
Member Author

Previously you couldn't have a non default non bold color. I could just surround the "input prints" with unconditional bold and things would be the same as before. I guess the reason things are a bit awkward is that colors and font style are put together in the same dict and the API has been so that you chose a key in that dict. A function with keyword arguments that return the full formatting strig might jave worked better.

Regarding light colors and bold, it is terminal dependent.

@KristofferC
Copy link
Member Author

Ref https://kristofferc.github.io/OhMyREPL.jl/latest/internals/ansitoken.html for how I do color printing in a package.

@StefanKarpinski
Copy link
Member

A better API is much needed.

@KristofferC
Copy link
Member Author

KristofferC commented Sep 14, 2016 •
edited
Loading

For now, I have decided to punt on more customization to input/output colors than what is currently available with ENV.

As the PR is right now it does not change the current state i.e:

  • If a user defines a color in an ENV variable for input/output it will be printed in bold.
  • If a user sets the color to :default it will be printed in non bold default color.

@KristofferC KristofferC changed the title Do not print in bold in print_with_color + other color tweaks WIP: Do not print in bold in print_with_color + other color tweaks Sep 21, 2016
@KristofferC KristofferC force-pushed the kc/little_less_bold branch 4 times, most recently from 32c1a31 to 1e20eb5 Compare September 21, 2016 21:08
@KristofferC KristofferC changed the title WIP: Do not print in bold in print_with_color + other color tweaks Do not print in bold in print_with_color + other color tweaks Sep 21, 2016
@KristofferC
Copy link
Member Author

Will repoen this because the new attempt is sufficiently different for the discussion here to not be very usable.

@KristofferC KristofferC deleted the kc/little_less_bold branch September 22, 2016 11:34
@KristofferC KristofferC mentioned this pull request Sep 22, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@KristofferC @andreasnoack @tkelman @StefanKarpinski